-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
5853 plot annotations prototype #6000
Conversation
…t-annotations-prototype
@akhenry @shefalijoshi Another round of changes. Annotations should now be only be editable/viewable in fixed time mode. Also, I've generally improved plot performance in fixed time mode. Please double check that plot performance is similar to |
This is looking really great, thank you! In testing this I think we maybe want to allow annotations in real-time mode if the plot is paused. Fine to do that in a followup though if it's a lot of work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have done some testing of this against real telemetry points in the viper test environment and discovered that Yamcs parameters cannot be annotated because the domain objects are immutable. Our implementation appears to be trying to mutate the telemetry points themselves?
I am unable to add a tag to an overlay plot containing a Yamcs telemetry point and I get the following error:
Yes, we have a item called The mutation is happening here. Note we had this with the Notebook tags too, but the notebook domain objects weren't immutable. |
For now, let's defer the problem of observing for annotation changes on immutable objects. We'll need to come up with a clever solution though... |
…t mutate if immutable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found a few minor issues in testing, will file followups. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes #5853
Describe your changes:
All Submissions:
Author Checklist
Reviewer Checklist